-
Notifications
You must be signed in to change notification settings - Fork 87
refactor!: Base combo box styles #8932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e01831c
to
d63d974
Compare
3928b71
to
5195b5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased the PR on top of the updated base-styles
branch.
ee5b8fe
to
ce4b0e6
Compare
5195b5d
to
94d978b
Compare
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
d599582
to
0d9c9b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased again. Some tests are failing, please do not merge until I fix those:
packages/combo-box/test/keyboard-lit.generated.test.js:
❌ keyboard > scrolling items > should scroll up after reaching the first visible item
AssertionError: expected 1 to deeply equal 2
+ expected - actual
-1
+2
at n.<anonymous> (packages/combo-box/test/keyboard-lit.generated.test.js:484:54)
❌ keyboard > scrolling items > should scroll to first visible when navigating down above viewport
AssertionError: expected 5 to deeply equal 6
+ expected - actual
-5
+6
at n.<anonymous> (packages/combo-box/test/keyboard-lit.generated.test.js:497:54)
❌ keyboard > scrolling items > should scroll to first visible when navigating up above viewport
AssertionError: expected 3 to deeply equal 4
+ expected - actual
-3
+4
at n.<anonymous> (packages/combo-box/test/keyboard-lit.generated.test.js:506:54)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Combo-box tests are passing now, other test failures are unrelated to this PR.
move declaration higher in the DOM hierarchy
|
I fixed the CSS lint errors directly in the |
@web-padawan, I’m not sure about the other test failures, if those are the ones you deemed unrelated to this PR. |
There are some multi-select-combo-box chip tests failing, I'll look into fixing those separately. Let's merge this PR. |
Co-authored-by: Jouni Koivuviita <jouni@vaadin.com>
Co-authored-by: Jouni Koivuviita <jouni@vaadin.com>
New and changed base style properties
--_vaadin-icon-checkmark
--_vaadin-icon-chevron-down
New Combo Box base style
Supported custom properties
--vaadin-combo-box-spinner-color
--vaadin-combo-box-overlay-width
--vaadin-item-border-radius
--vaadin-item-checkmark-color
--vaadin-item-gap
--vaadin-item-height
--vaadin-item-overlay-padding
--vaadin-item-padding